Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.Numerics;
using System.Runtime.InteropServices;

BenchmarkSwitcher.FromAssembly(typeof(Tests).Assembly).Run(args);

[DisassemblyDiagnoser]
public class Tests
{
    [Params(1000, 10000, 100000, 1000000)]
    public int N;

    string s1, s2, s3;

    [GlobalSetup]
    public void Setup()
    {
        s1 = "1" + new string('0', N);
        s2 = "1" + new string('0', N - 1) + "1";
        s3 = new string('9', N - 1);
    }

    [Benchmark]
    public BigInteger PowerOfTen() => BigInteger.Parse(s1);

    [Benchmark(Baseline = true)]
    public BigInteger PowerOfTenPlusOne() => BigInteger.Parse(s2);

    [Benchmark]
    public BigInteger PowerOfTenMinusOne() => BigInteger.Parse(s3);
}

BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3085/23H2/2023Update/SunValley3)
13th Gen Intel Core i5-13500, 1 CPU, 20 logical and 14 physical cores
.NET SDK 9.0.100-alpha.1.23615.4
  [Host]     : .NET 9.0.0 (9.0.23.61410), X64 RyuJIT AVX2
  Job-KEBJOQ : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
  Job-VLAEBK : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2

I updated the benchmark target to the latest commit.

I updated the benchmark target to the latest commit.

Thanks! Slowly getting through stuff here and just want to ensure no conflicts/accurate numbers/etc as they go through.

Looks like there are some related CI failures that need to get resolved.

Tests fail when parseTest and ToStringTest are run in parallel. I think RunWithFakeThreshold is the cause of failure.
I set DisableParallelization so that RunWithFakeThreshold is not executed in parallel.

I was able to reproduce the error with the following code.

Number.s_naiveThreshold = 0;
BigInteger.Parse("99000000000");

@huoyaoyuan, if you have some time I'd appreciate if you could give this a once over as well, given you've also been touching/updating the parsing logic.

I'd ideally like to get #95402 done first and I know there's another PR out, but you recently changed that one back to draft

I'd ideally like to get #95402 done first and I know there's another PR out, but you recently changed that one back to draft

I've done the code update and measuring the performance. I'd expect to make it ready this weekend.

@kzrnm, sorry for the delay on this. Could you share updated numbers, just so we can confirm nothing has regressed given all the other work that's been done?

After that, I think we can get this merged.

@tannergooding I ran benchmarks on the latest branch.

BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3810/23H2/2023Update/SunValley3)
13th Gen Intel Core i5-13500, 1 CPU, 20 logical and 14 physical cores
.NET SDK 9.0.100-preview.5.24307.3
  [Host]   : .NET 9.0.0 (9.0.24.30607), X64 RyuJIT AVX2
  ShortRun : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2

Job=ShortRun  IterationCount=3  LaunchCount=1  
WarmupCount=3  

@tannergooding Regressions:
Linux x64: dotnet/perf-autofiling-issues#37866, dotnet/perf-autofiling-issues#37802
Windows x64: dotnet/perf-autofiling-issues#37852 (BigInteger regressions), dotnet/perf-autofiling-issues#37817 (BigInteger regressions)
Linux arm64: dotnet/perf-autofiling-issues#38039

Mono related regressions:

• [Perf] Linux/arm64: 2 Regressions on 7/3/2024 6:29:56 PM perf-autofiling-issues#37939
• [Perf] Linux/x64: 3 Regressions on 7/3/2024 8:41:35 PM perf-autofiling-issues#37879
  Mono related improvements:
• [Perf] Linux/arm64: 2 Improvements on 7/3/2024 6:29:56 PM perf-autofiling-issues#37943
• [Perf] Linux/x64: 3 Improvements on 7/3/2024 8:41:35 PM perf-autofiling-issues#37884

Just noting on the regressions above that many of them are for very small inputs, like 123. I've raised that these aren't "representative" of the cases we care about here since if users care about perf for numbers that are less than or equal to 128-bits, then we have existing dedicated types they can use.

Rather, we want to ensure that BigInteger really shines for values above this cutoff. -- There is notably some consideration that we also don't want to overly regress the perf of BigInteger values with 128 or less bits, just that its not as important. For a case like Parse we could likely have a fast path that does something like if (stringLength < 40) { ParseAsInt128 } else { ParseAsBigInt }, since any string with this few digits must fit into Int128 (and a corresponding cutoff for hex numbers, etc). This would likely fix the regression without hurting the improved cases.